Add environment vars expansion and pre defined git hashes in the yaml #120
Conversation
tests/test_commons_docker.py
Outdated
| @@ -180,7 +180,8 @@ def test_docker_build_caches_pkg_installation(EXPORTER, config, | |||
| capture_output=True) | |||
|
|
|||
| ls = out.stdout.decode() | |||
| expected = ('environment.yml\nfast-pipeline.tar.gz\n' | |||
|
|
|||
| expected = ('env.yaml\nenvironment.yml\nfast-pipeline.tar.gz\n' | |||
There was a problem hiding this comment.
can you add two more tests?
let's check the content of the env.yaml, one when a user already has one (then the content should be the existing user values + the default ones) and another one when there is no user env.yaml and we create it
There was a problem hiding this comment.
Yes but can you please share some thoughts on how these test projects are created, it doesnt seem any of them uses a git repo etc ? and how do you go about editing the cases (ie keep the same test project but one case with the env file, one case with it being a repo, one case - the current setup, without needing 3 separate projects).
| check=True, | ||
| capture_output=True) | ||
|
|
||
| env_contents = out.stdout.decode() |
There was a problem hiding this comment.
@edublancas this file only has {} as contents instead it should have build_time and the git placeholders it does have the build time locally pre sending to docker and not the placeholders but im kinda lost in all the indirection where is that stuff being injected / cleared.
until docker.py image = build_image the env.yaml from env_file_path has the correct build_time but is still missing the git values and then the env.yaml that results in the docker image is empty. You can put a breakpoint and see it shouldnt be (has build time, still doesnt have git data even though i added the git init and it's a repo now...) So im a bit out of ideas for now.
|
hey @feribg, I fixed and merged the PR. Please take the version in master for a spin and let me know if it works for you. If so, I'll make a release. Note: I saw you had a |
|
@edublancas thanks i will take a look and try to use the master one and report back. Yes I just added that one but didnt end up implementing it the idea being is to act as a git hash if it's a git repo but I dont really need it, figured it might be useful. |
|
great, I also released a new ploomber version that contains your changes. |
|
@edublancas doesn't look like its working correctly.
Seems like it keeps looking for .git for whatever reason and its a bit tricky to debug it in AWS batch. the |
|
Since ploomber implements the logic for expanding these values, it's possible to replicate this exclusively with ploomber, right? No AWS Batch, no docker. Based on the error message, looks like there is a bug that's erroring because there is no repository, so probably we need to add some extra logic to ploomber that checks if those placeholders are defined already and skip checking for a git repository. Do you have time to submit a PR? |
Describe your changes
Issue ticket number and link
Closes #116
Checklist before requesting a review